Skip to content

fix(mavlink): Allowing custom modes to request offboard setpoints via the external mode registration#26949

Merged
Jaeyoung-Lim merged 6 commits intoPX4:mainfrom
jonas-eschmann:offboard_setpoints
Apr 13, 2026
Merged

fix(mavlink): Allowing custom modes to request offboard setpoints via the external mode registration#26949
Jaeyoung-Lim merged 6 commits intoPX4:mainfrom
jonas-eschmann:offboard_setpoints

Conversation

@jonas-eschmann
Copy link
Copy Markdown
Contributor

@jonas-eschmann jonas-eschmann commented Apr 2, 2026

Solved Problem

Fixes #26768

Solution

This PR implements Alternative 1

        ┌─────────────────┐
        │  Custom Mode    │
        └─────────────┬───┘
                      │ register_ext_component_request.request_offboard_setpoints
        ┌─────────────┼──────────┐
        │ Commander   |          │
        │ ┌───────────▼────────┐ │
        │ │ ModeManagement     │ │
        │ └────────────────────┘ │
        └────────┬───────────────┘
                 │ vehicle_status.accepts_offboard_setpoints
                 ▼
        ┌─────────────────┐    MAVLink: SET_POSITION_TARGET_LOCAL_NED
        │ MavlinkReceiver │◀───────────────────────────────────────────
        └────────┬────────┘
                 │ trajectory_setpoint
                 ▼
        ┌─────────────────┐
        │  Custom Mode    │
        └─────────────────┘

Changelog Entry

For release notes:

Feature/Bugfix #26768 Allowing modes that use the external mode registration to receive offboard setpoints

Alternatives

Approach Pros Cons
GrooveWJH PR: The Mavlink Receiver observes the external modes through the request topics No modifications to vehicle status The Mavlink receiver gets entangled with the mode registration
Alternative 1: request_offboard_setpoints in the external mode registration. This needs to be reflected in the vehicle_status because that is the main source determining the behavior of the mavlink_receiver.cpp Cleanest long-term solution   adds flag to vehicle_status
Alternative 2: Make mavlink_receiver.cpp always emit trajectory_setpoint for external mode IDs one-line fix  If there is ever an in-source mode that uses the external mode registration but also generates trajectory_setpoint itself, they could be conflicting. AFAIK currently only mc_raptor and mc_nn_control use the external mode registration from the inside and for external modes it does not matter anyways, so this is not an issue

I believe Alternative 1 is the the right approach here. It unambiguously implements the forwarding while neither entangling mavlink_receiver.cpp with the mode registration (in contrast to GrooveWJH PR) nor adding assumptions about the NAVIGATION_STATE_EXTERNAL{1-8}

Context

External modes like mc_raptor and mc_nn_control that use the external mode registration benefit greatly from being able to receive offboard setpoints via MAVLink

@jonas-eschmann jonas-eschmann mentioned this pull request Apr 2, 2026
Open
Jaeyoung-Lim
Jaeyoung-Lim reviewed Apr 3, 2026
View reviewed changes
Comment thread src/modules/mavlink/mavlink_receiver.cpp
Pedro-Roque
Pedro-Roque reviewed Apr 5, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@Pedro-Roque Pedro-Roque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I prefer this version much more... Also perhaps it could be a good idea to aggregate multiple requests to change message versions before doing so for adding one extra field, unless urgent. Otherwise we will end up with large version numbers soon.

Comment thread src/modules/commander/ModeManagement.hpp
@jonas-eschmann
Copy link
Copy Markdown
Contributor Author

jonas-eschmann commented Apr 5, 2026

@Pedro-Roque

Not sure if I prefer this version much more... Also perhaps it could be a good idea to aggregate multiple requests to change message versions before doing so for adding one extra field, unless urgent. Otherwise we will end up with large version numbers soon.

I agree about the message versions. I just bumped the version because otherwise the CI tests complain.

@DronecodeBot
Copy link
Copy Markdown

DronecodeBot commented Apr 8, 2026

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-dev-call-apr-08-2026-team-sync-and-community-q-a/48785/1

@mrpollo mrpollo requested a review from Jaeyoung-Lim April 8, 2026 15:29
@Pedro-Roque Pedro-Roque requested a review from dakejahl April 8, 2026 17:11
@GrooveWJH
Copy link
Copy Markdown

GrooveWJH commented Apr 9, 2026

@Pedro-Roque

Not sure if I prefer this version much more... Also perhaps it could be a good idea to aggregate multiple requests to change message versions before doing so for adding one extra field, unless urgent. Otherwise we will end up with large version numbers soon.

I agree about the message versions. I just bumped the version because otherwise the CI tests complain.

Yes, in fact, I was also quite conflicted about whether the message version should be changed when modifying the code. In any case, I hope everyone present can discuss and reach a conclusion as soon as possible. After the merge, I will immediately apply this technology to industry-level scenarios, which was my original intention when I first submitted the PR.

bkueng
bkueng approved these changes Apr 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

Longer-term I think it needs a better solution though. Rather than mc_raptor registering as external mode, it should register itself as a controller with the setpoint types it accepts. Then adding a mechanism to select the controller(s) at runtime (either by the mode, or something else).
This would make it more flexible allowing internal or external modes to use different controllers.

@Jaeyoung-Lim Jaeyoung-Lim enabled auto-merge (squash) April 13, 2026 16:05
@Jaeyoung-Lim Jaeyoung-Lim merged commit abc756b into PX4:main Apr 13, 2026
73 checks passed
@Jaeyoung-Lim
Copy link
Copy Markdown
Member

Jaeyoung-Lim commented Apr 13, 2026

Thanks @bkueng !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Pedro-Roque Pedro-Roque Pedro-Roque left review comments

@bkueng bkueng bkueng approved these changes

@Jaeyoung-Lim Jaeyoung-Lim Jaeyoung-Lim approved these changes

@dakejahl dakejahl Awaiting requested review from dakejahl

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] MAVLink/MAVROS Offboard setpoints ignored when OFFBOARD is replaced by external mode

6 participants

@jonas-eschmann @DronecodeBot @GrooveWJH @Jaeyoung-Lim @bkueng @Pedro-Roque